[BUGFIX] Ignore $ref siblings & abort on infinite-loop references #437
Conversation
From the spec [1]:
An object schema with a "$ref" property MUST be interpreted as a
"$ref" reference. The value of the "$ref" property MUST be a URI
Reference. Resolved against the current URI base, it identifies the
URI of a schema to use. All other properties in a "$ref" object MUST
be ignored.
[1] https://tools.ietf.org/html/draft-wright-json-schema-01#section-8
Schemas containing $ref MUST ignore all other properties, so just return the reference target directly. Because some circular references may result in an infinite loop, keep track of the objects that have been dereferenced during the current call to SchemaStorage and abort if the same object is encountered more than once.
|
@shmax @bighappyface @mathroc The check in this PR doesn't have the same performance implications, because it only needs to check whether objects are the same instance - but this approach will not work in 5.x.x, because 5.x.x deliberately preserves properties from both the referenced schema and the referer, and returns an entirely new object. Alternatively, if any of you have an idea for how to do this for 5.x.x in a saner way performance-wise, ideas are welcome... |
|
I don't think it's very important to backport the infinitely-looping check to 5.x, working on 6.x and releasing seems a more appropriate use of time. especially as an upgrade to 6.x should be painless, users hitting this bug should be able to upgrade easily once released (or use dev-master before that) |
|
I agree with @mathroc. If people haven't complained of this bug before now then they're probably not hitting it, and there's no harm in waiting for 6. |
|
Agreed on focusing on v6.x.x |
|
OK - I won't worry about fixing the loop in 5.x.x then :-). This PR is ready for review whenever you guys feel like it. |
|
👍 |
What
$refis present (fixes Sibling properties of $ref are not ignored #436).Why
$refMUST be ignoredThis PR will not be backported. #436 is definitely a bug, but fixing it results in a significant change in behavior which some users may be relying on (e.g. as a hack to implement inheritance), and as such it should be merged as major-version change.
Thanks to @johannes-see for finding the bug (#435).